Skip to content

feat(bench): add managed nightly RPC benchmarks - #3944

Merged
danielntmd merged 2 commits into
mainfrom
danielntmd/benchmark-endpoints
Aug 25, 2026
Merged

feat(bench): add managed nightly RPC benchmarks#3944
danielntmd merged 2 commits into
mainfrom
danielntmd/benchmark-endpoints

Conversation

@danielntmd

@danielntmd danielntmd commented Aug 14, 2026

Copy link
Copy Markdown
Contributor
  • Adds a nightly.sh run script that handles corpus generation/removal and warmup/measured k6 runs.
  • Publish a dedicated benchmark image.
  • Nightly.json catalog validation in CI.

@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.42%. Comparing base (d5d68c0) to head (1909584).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3944      +/-   ##
==========================================
- Coverage   74.63%   74.42%   -0.21%     
==========================================
  Files         462      462              
  Lines       40989    41061      +72     
==========================================
- Hits        30592    30561      -31     
- Misses       8294     8393      +99     
- Partials     2103     2107       +4     
Flag Coverage Δ
jsonv2 77.17% <ø> (-0.19%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from a70bc65 to dcb6bf9 Compare August 17, 2026 15:05
@danielntmd
danielntmd changed the base branch from danielntmd/benchmark-image to dat/bench-full-flow August 17, 2026 17:46
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch 5 times, most recently from a19dee7 to 17d0c39 Compare August 17, 2026 22:19
@danielntmd danielntmd changed the title feat(bench): benchmark supported RPC endpoints feat(bench): add managed RPC benchmark flow Aug 17, 2026
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from 17d0c39 to 72969ea Compare August 18, 2026 04:30
Base automatically changed from dat/bench-full-flow to main August 18, 2026 09:14
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from 72969ea to a39478c Compare August 18, 2026 19:26
@danielntmd danielntmd changed the title feat(bench): add managed RPC benchmark flow feat(bench): add managed nightly RPC benchmarks Aug 18, 2026
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from a39478c to 49fe138 Compare August 18, 2026 20:04
@danielntmd
danielntmd marked this pull request as ready for review August 18, 2026 21:36
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from 49fe138 to 70d5234 Compare August 18, 2026 21:36
@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @danielntmd's task in 8m 14s —— View job


Review: managed nightly RPC benchmarks

  • Read repo guidelines (no root CLAUDE.md in this checkout)
  • Review shell scripts (gen-all.sh, run-all.sh, nightly.sh, test-case-artifacts.sh)
  • Review Dockerfile, compose, workflow
  • Review run.js and static viewer
  • Post inline findings + summary

Solid, careful work — atomic staged generation with mv -T, checksum-verified reuse, case-ID validation against path escape, signal forwarding to the live k6 process, --summary-export validated before a case is called passed, and a genuinely useful test script that covers the failure paths (empty summary, failed warmup, missing corpus, wrong snapshot, mutated corpus, TERM propagation). The Docusaurus keep_files: true deploy makes the docs/static viewer + Argo-published runs/ coexist correctly. No correctness bug that would corrupt a measurement or leak resources.

The findings below are mostly about operability of the reuse scheme — what happens on the days when inputs change.

Blocker

None.

Important

  1. generate doesn't preflight (nightly.sh:49) — manifests stamp SNAPSHOT_ID/SHA256/BLOCK_NUMBER as provenance that was never verified against the node (only rpcVersion is cross-checked). Every reuse decision and published result then trusts it. compose.yaml:20 passes EXPECTED_CHAIN_ID to the corpus service, where it's currently unused — the intent looks like it was already there.
  2. An incompatible case is terminal (gen-all.sh:214-223) — reuse fails and generate_managed refuses to write over an existing dir, so a snapshot rotation (a routine event here) fails all 23 cases until someone manually clears the PVC. Keying storage by identity hash (<case>/<hash>/) or adding a supported prune/regenerate path would fix it.
  3. Corpus identity is the raw argv vector (gen-all.sh:62-67) — --concurrency is included even though it can't affect content (newSeededRand(seed, i) + WithMaxGoroutines), and --count 2 vs --count=2 mismatch, so a README-generated PVC and a nightly-generated one are mutually unusable.
  4. WARMUP_MAX_DURATION is a failure trigger, not a bound (run-all.sh:168-184 + run.js:23-25) — hitting the 10m cap fails the count==N threshold, so the case is failed and measurement is skipped. Likely to bite traceBlockTransactions/traceTransaction/getStorageProof at 200 sequential iterations over 10k sampled mainnet blocks.
  5. summary.json records what, not how (run-all.sh:302-316) — no --vus/--duration and no per-case corpus identity, so dashboard trends aren't comparable across a VUS or corpus --count/--seed change.

Nit

  1. curl in rpc_result has no --connect-timeout/--max-time (nightly.sh:15-23) — a stalled node hangs the nightly until Argo's deadline.
  2. Empty/malformed corpus → % 0NaN index → every VU POSTs undefined (run.js:36); reads as 100% check failures instead of "bad corpus".
  3. Stale per-case artifacts for removed catalog entries survive in a reused RESULTS_DIR and get published (run-all.sh:139-142).
  4. verify_managed's single error message covers both a real mismatch and a missing/corrupt corpus.json (gen-all.sh:112-117), pointing the operator at the wrong thing.
  5. chown k6:k6 /corpus /results is defeated by root-owned PVC mounts (Dockerfile.benchmark:15-18) — worth documenting the required fsGroup.
  6. entry.summary from history.json is unvalidated before fetch (index.html:123-131), and render() assumes run/juno/snapshot exist.
  7. Not commented inline: WARMUP_MAX_DURATION isn't validated the way WARMUP_ITERATIONS/STRICT_CHECKS are (run-all.sh:32-42); .staging/ dirs from a killed generation are never reaped (gen-all.sh:124); run-all.sh reports EXPECTED_CHAIN_ID/SNAPSHOT_BLOCK_NUMBER as juno.chainId/juno.blockNumber without asking the node (safe under nightly.sh, misleading standalone); and 23 cases × (warmup + 30s) can approach hours, so the Argo job needs a deliberate deadline.

test-case-artifacts.sh doubling as driver and as the stub for gen-all/run-all/curl/k6/corpus-gen (dispatching on basename $0 + STUB_*) is effective but dense; a small stubs/ dir would read better as this grows.

Note: I reviewed by reading — the sandbox here blocked running bash -n, the test script, and docker compose config, so I didn't independently execute the validation listed in the PR description.
• branch danielntmd/benchmark-endpoints

Comment thread bench/rpc/nightly.sh Outdated
Comment thread bench/rpc/gen-all.sh Outdated
Comment thread bench/rpc/gen-all.sh Outdated
Comment thread bench/rpc/run-all.sh Outdated
Comment thread bench/rpc/run-all.sh Outdated
Comment thread bench/rpc/nightly.sh Outdated
Comment thread bench/rpc/run.js Outdated
Comment thread bench/rpc/run-all.sh Outdated
Comment thread bench/rpc/gen-all.sh Outdated
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from 2336b7f to 1a40181 Compare August 21, 2026 04:01
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @danielntmd's task in 4m 20s —— View job


Re-review: managed nightly RPC benchmarks

  • Diff the new push against origin/main and the previously reviewed revision
  • Verify the items raised last round
  • Review the new changes for regressions
  • Post inline findings + summary

Still 388 insertions, 0 deletions — purely additive, gen-all.sh / run-all.sh / run.js / bench/rpc/README.md untouched. Confirmed against origin/main.

This push is one change, and it's the one I'd have picked: command is now part of the reuse key. generate_case stages a command file next to the corpus (internal/corpus.sh:16) and the new corpus_is_current (:26-33) compares it alongside .meta.count/.meta.seed. Repointing a case at a different subcommand now regenerates instead of silently publishing the old method's numbers under the new ID — the coverage regression from dropping the manifest is closed. The [[ -f $case_dir/command ]] guard is a nice detail: corpora written by an older image fail the test and regenerate once rather than being trusted, so the migration is self-healing rather than something an operator has to notice.

Nothing regressed. mv -T staging, the snapshot-keyed layout, SNAPSHOT_SHA256 validation, and the missing-corpus failure path are unchanged. I compared the two catalogs' key sets by hand — still identical at 23, which you've said is expected to drift and I'm taking as settled.

The two "important" items below are both things the removed manifest used to cover and the restructure didn't carry forward — same shape as the command gap you just fixed.

Blocker

None.

Important

  1. A failed case publishes the previous run's numbers (nightly.sh:45) — $summary is never removed, and three paths reach the next iteration without k6 writing it (missing corpus, failed warmup, k6 crash). With the aggregated summary.json gone, "which <case>.json files exist" is the only status signal a consumer has, and a stale file makes it lie. run-all.sh had the rm -f for exactly this; it didn't survive.
  2. Reuse key omits the node's RPC spec version (internal/corpus.sh:26-33) — the snapshot digest keys the directory, but the node is a separate input. A Juno upgrade that bumps the spec version under an unchanged snapshot reuses corpora shaped for the old spec. corpus-gen already stamps meta.rpcVersion (cmd/corpus-gen/main.go:128, :241) and preflight already has the connection open.

Nit

  1. Pruning is skipped whenever any case fails (internal/corpus.sh:74-79) — the early return 1 sits in front of prune_removed_cases, so one transient generation failure out of 23 means removed catalog entries keep their PVC space until a night where everything succeeds.

Not worth another inline comment — all previously raised and answered: the catalog-read-⇒-rm -rf shape (you're right that CI validates the catalog's contents; what stays uncovered is the file being absent or at the wrong path inside the image, which is the same gap as the untested image), argument dispatch and the output contract vs argocd#9780, old snapshot trees never being reclaimed, the workflow's missing go.mod/go.sum build-input paths, and the absent bench/rpc/nightly/ docs. On the accepted CI tradeoff, the one concrete thing it leaves open is --summary-mode=disabled (nightly.sh:65) against the pinned k6 1.7.1 — if that value isn't accepted, every warmup fails and all 23 measurements are skipped. Worth confirming by hand once before the first nightly.

Note: as in all five prior rounds, the sandbox blocked bash -n, test-nightly.sh, jq, and docker — this is a reading review, and I executed nothing. Web access was also unavailable, so I could not settle the --summary-mode question myself.
· branch danielntmd/benchmark-endpoints

Comment thread bench/rpc/nightly/nightly.sh
Comment thread bench/rpc/nightly/internal/corpus.sh
Comment thread bench/rpc/nightly/internal/corpus.sh
Comment thread bench/rpc/nightly/internal/corpus.sh

@infrmtcs infrmtcs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version is much better than the previous one. I've reviewed ~80% of this PR and left these comments, I think the remaining can be reviewed after we address these.

Comment thread bench/rpc/corpus/nightly.json
Comment thread bench/rpc/nightly/warmup.js Outdated
Comment thread bench/rpc/nightly/Dockerfile Outdated
Comment thread bench/rpc/nightly/Dockerfile
Comment thread bench/rpc/nightly/internal/corpus.sh
Comment thread bench/rpc/nightly/nightly.sh Outdated
Comment thread .github/workflows/rpc-benchmark.yaml Outdated
Comment thread .github/workflows/rpc-benchmark.yaml Outdated
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from 1a40181 to 2ea3475 Compare August 21, 2026 15:14
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @danielntmd's task in 6m 54s —— View job


Re-review: managed nightly RPC benchmarks

  • Diff the new push against origin/main and the previously reviewed revision
  • Check the items @infrmtcs raised this morning
  • Review the new changes for regressions
  • Post inline findings + summary

Still 344 insertions, 1 deletion — the only deletion is the missing trailing newline on .dockerignore. gen-all.sh, run-all.sh, run.js, and bench/rpc/README.md remain untouched. Confirmed against origin/main.

Every item @infrmtcs raised is addressed, and the catalog change in particular does more work than it looks like:

  • Catalog back to string commands. nightly.json values are now "blockNumber --count=100000 --seed=1", so no JSON-shape parsing and no argv reconstruction. Three consequences fall out for free: corpus_is_current comparing one string now covers command and count and seed (both gaps I raised in the last two rounds, closed by deletion rather than addition); gen-all.sh can consume nightly.json directly since it already read -ras the value; and test-nightly.sh went back to the same validator gen-all.sh uses. I ran that jq expression against the catalog locally — passes, and both catalogs still carry the same 23 IDs.
  • warmup.js dropped. Warmup is run.js with --vus 1 --iterations N, which is what was asked for. Worth noting it also retires the unset K6_VUS K6_DURATION dance — CLI flags outrank env vars in k6's precedence order, so that was indeed unnecessary. And the round-one warmup-cap issue stays closed for a different reason than before: no WARMUP_TARGET threshold exists any more, so hitting shared-iterations' default 10m maxDuration truncates the warmup silently instead of failing the case and skipping the measurement.
  • Dockerfile uses make corpus-gen, with Makefile copied into the builder.
  • .dockerignore guards generated corporabench/rpc/corpus/* plus !bench/rpc/corpus/*.json. I checked the negation still admits nightly.json and all.json (Docker's matcher, unlike gitignore's, re-includes under an excluded parent), and that the build/ rule doesn't touch COPY --from=builder.
  • Build and publish merged into one job, with the JFrog/login steps and push: gated on refs/heads/main. That removes the cross-runner cache round-trip you were worried about, and a fork PR never reaches the OIDC steps.

I traced the container paths again — CORPUS_GEN/app/build/corpus-gen, RUN_SCRIPT/app/bench/rpc/run.js, catalog → /app/bench/rpc/corpus/nightly.json, CI_CONFIG/app/bench/rpc/nightly/ci.json — and they line up with the COPY layout. nightly.sh is mode 100755, so the new ENTRYPOINT works. run.js still exports no options, so --config ci.json isn't outranked. No correctness bug that would corrupt a measurement.

Blocker

None.

Important

  1. --summary-mode=disabled is now redundant and still unverified (nightly.sh:60) — the subshell already redirects k6's stdout to /dev/null, which is where the summary goes. The flag buys nothing, but if disabled isn't accepted by the pinned 1.7.1 (k6 documents compact/full/legacy; I couldn't check, no web access and CI runs nothing) then every warmup fails and all 23 measurements are skipped. Deleting the line removes the risk at zero cost.
  2. ci.json thresholds are absolute, and the off-switch went with them (ci.json:5-8) — checks: rate==1 / http_req_failed: rate==0 fail a case on one bad response. That's a red nightly for a single dropped connection, and a permanently red case for anything that errors structurally on some sampled inputs (getStorageProof outside the proof window, traceTransaction on old transactions). STRICT_CHECKS used to default to 0; thresholds can't be set from the environment and nightly.sh doesn't expose --no-thresholds, so there's now no way to run without them. The exported summary is written either way, so the only thing they buy is the exit code.
  3. The PR trigger gained a build input it doesn't watch (rpc-benchmark.yaml:4-7) — Makefile is now COPY'd and make corpus-gen is the build command, so the recipe at Makefile:46-48 is part of the image contract. Plus go.mod/go.sum/starknet-p2p-specs/go.mod, still unwatched. Breakage lands on main, where push: true.

Nit

  1. The reuse decision no longer reads corpus.json (internal/corpus.sh:24-28) — a case dir with a valid command and a missing/truncated corpus is "current" forever: generate reuses, benchmark fails it with missing corpus, repeat nightly. mv -T makes it hard to reach from a clean run, so this is about partial PVC restores and hand-edits; [[ -s $case_dir/corpus.json ]] restores the self-healing.
  2. WARMUP_ITERATIONS is the last unvalidated knob (nightly.sh:33) — abc → k6 rejects --iterations → all 23 warmups fail → all measurements skipped. Better than the old NaN (it's a loud k6 error now), but it still costs the night.
  3. COPY Makefile precedes RUN go mod download (Dockerfile:6-9) — any Makefile edit re-pulls the module graph.

Not worth another inline comment, all previously raised and answered: the catalog-read-⇒-rm -rf shape in prune_removed_cases, argument dispatch and the output contract vs argocd#9780, old snapshot trees never reclaimed, rm -f "$summary" before each case, pruning skipped when any case fails, .staging/ never reaped, and the absent bench/rpc/nightly/ docs — nothing in the repo still names SNAPSHOT_SHA256, CORPUS_ROOT, RESULTS_DIR, WARMUP_ITERATIONS, CONCURRENCY, or RPC_TIMEOUT, which is the whole Argo contract.

Note: as in all six prior rounds, the sandbox blocked bash -n, test-nightly.sh, and docker. I executed only the catalog's jq validation expression; everything else is a reading review, and web access was unavailable so I could not settle --summary-mode myself.
· branch danielntmd/benchmark-endpoints

Comment thread bench/rpc/nightly/nightly.sh Outdated
Comment thread bench/rpc/nightly/ci.json
Comment thread .github/workflows/rpc-benchmark.yaml
Comment thread bench/rpc/nightly/internal/corpus.sh
Comment thread bench/rpc/nightly/nightly.sh
Comment thread bench/rpc/nightly/Dockerfile
- Adds a nightly runner for corpus reconciliation and warmup/measured k6 runs.
- Publish a dedicated benchmark image.
- Nightly.json catalog validation in CI.
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from 2ea3475 to 6fd6d61 Compare August 21, 2026 15:44
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @danielntmd's task in 3m 49s —— View job


Re-review: managed nightly RPC benchmarks

  • Diff the new push against origin/main and the previously reviewed revision
  • Verify the items raised last round
  • Review the new changes for regressions
  • Post inline findings + summary

342 insertions, 1 deletion — the deletion is still just the missing trailing newline on .dockerignore. gen-all.sh, run-all.sh, run.js, and bench/rpc/README.md remain untouched. Confirmed against origin/main.

Both items I'd have picked from last round landed:

  • --summary-mode=disabled is gone (nightly.sh:55-61). The subshell's >/dev/null was already doing the work, and this retires the one concrete risk the untested image was carrying — a rejected flag would have failed all 23 warmups and skipped every measurement.
  • COPY Makefile ./ moved below RUN go mod download (Dockerfile:8-10), so Makefile edits no longer re-pull the module graph.

I re-traced the build while checking that second one: Makefile is not matched by any .dockerignore rule, so COPY Makefile ./ resolves; Makefile:46-48's corpus-gen recipe has no prerequisites outside what the builder stage copies and writes to build/corpus-gen, which is where Dockerfile:22 reads from; and the build/ ignore rule only filters the host context, not COPY --from=builder. nightly.sh is 100755 and test-nightly.sh is 100755, so the ENTRYPOINT and the CI step both work. Nothing regressed — mv -T staging, the snapshot-keyed layout, SNAPSHOT_SHA256 validation, and the missing-corpus failure path are unchanged.

Correction

My claim last round that getStorageProof "errors structurally on some sampled inputs" was wrong. get_storage_proof.go:45-46 pins BlockID: "latest" with a comment saying exactly why — proofs are only served near head, and the sampled trie members stay valid. That case is not a permanent-red candidate. The finding below stands on the transient-error argument alone.

Blocker

None.

Important

  1. K6_VUS/K6_DURATION now outrank ci.json (nightly.sh:69, inline) — new this push. k6's precedence is config file → script options → env vars → CLI flags, so those two sit above --config, not below it. Dropping warmup.js also dropped the unset K6_VUS K6_DURATION that guarded this. Nothing in this repo sets them, so it only bites if the Argo manifest does — but then the measured run silently isn't 50 VUs / 30s and nothing in the exported summary says so.

  2. ci.json thresholds are absolute and there's no off switch (ci.json:5-8) — unchanged. checks: rate==1 / http_req_failed: rate==0 fail a case on one bad response out of 50 VUs × 30s × 23 cases; benchmark() then marks it failed and nightly_run returns non-zero, so a single dropped connection reds the whole nightly and hands Argo a failed pod to retry — a retry that re-runs all 23 cases. STRICT_CHECKS used to default to 0; thresholds can't be set from the environment and nightly.sh doesn't expose --no-thresholds. Since k6 writes --summary-export regardless of threshold state, the only thing these buy is that exit code.

  3. The PR trigger doesn't watch the image's build inputs (rpc-benchmark.yaml:4-7) — unchanged, and this push makes it sharper: make corpus-gen is now the build command, so Makefile:46-48 is part of the image contract, alongside the still-unwatched go.mod / go.sum / starknet-p2p-specs/go.mod. Breakage lands on main, where push: evaluates true.

Nit

  1. The reuse decision no longer reads corpus.json (internal/corpus.sh:24-28) — a case dir with a valid command and a missing or truncated corpus is "current" forever: generate reuses, benchmark fails it with missing corpus, repeat nightly. mv -T makes it hard to reach from a clean run, so this is about partial PVC restores and hand-edits; [[ -s $case_dir/corpus.json ]] restores the self-healing.
  2. WARMUP_ITERATIONS is the last unvalidated knob (nightly.sh:33) — abc → k6 rejects --iterations → all 23 warmups fail → all measurements skipped.

Not worth another inline comment, all previously raised and answered: the catalog-read-⇒-rm -rf shape in prune_removed_cases, argument dispatch and the output contract vs argocd#9780, old snapshot trees never reclaimed, rm -f "$summary" before each case, pruning skipped when any case fails, .staging/ never reaped, and the absent bench/rpc/nightly/ docs — nothing in the repo names SNAPSHOT_SHA256, CORPUS_ROOT, RESULTS_DIR, WARMUP_ITERATIONS, CONCURRENCY, or RPC_TIMEOUT, which is the whole Argo contract.

Note: as in all seven prior rounds, the sandbox blocked bash -n, test-nightly.sh, and docker, and web access was unavailable. I read the Makefile, .dockerignore, and get_storage_proof.go to check the build path and the correction above; everything else is a reading review.
· branch danielntmd/benchmark-endpoints

Comment thread bench/rpc/nightly/nightly.sh
Comment thread bench/rpc/nightly/Dockerfile
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @danielntmd's task in 3m 2s —— View job


Re-review: managed nightly RPC benchmarks

  • Diff the new push against origin/main and the previously reviewed revision
  • Verify the items raised last round
  • Review the new changes for regressions
  • Post inline findings + summary

343 insertions, 2 deletions. The bench code is byte-identical to the revision I reviewed last round — the delta is exactly one line, and it isn't in bench/:

.github/workflows/build-image.yaml | 2 +-

That's commit 1909584be ("ci: publish main-tagged Juno image"), which adds a conditional juno:main tag to the production Juno image publish step. Two things worth stating plainly:

  • @infrmtcs's approval landed at 11:16 UTC; this commit is dated 21:02 UTC. The approved diff was the purely-additive, bench/-only one. This commit changes the workflow that publishes Juno's own image on every merge to main, which is a different blast radius from everything else here and isn't mentioned in the PR description.
  • It's presumably here because the Argo nightly needs a stable Juno reference to pull — which is exactly the thing I'd want confirmed against argocd#9780 rather than inferred.

I traced the mechanics and they're right: workflow_call inherits the caller's github context; deploy-dev-and-test.yml fires on merge_group / pull_request / push (main + v* tags) / workflow_dispatch, so only the main-branch push evaluates the condition true — PR builds and release tags leave :main alone. build-push-action splits tags on comma as well as newline, so the concatenated single-line form does produce two tags. cancel-in-progress is keyed on github.ref, so back-to-back merges cancel the older run and :main lands on the newer commit rather than racing backwards.

Blocker

None.

Important

  1. :main is mutable, and nothing records which build actually ran (build-image.yaml:65, inline) — new this push. Kubernetes defaults imagePullPolicy to Always only for :latest or an omitted tag; for :main it defaults to IfNotPresent, so a node with the image cached never re-pulls. The restructure already removed the aggregated summary.json that carried juno.version/imageDigest, and preflight is a bare starknet_chainId ping, so the published artifacts contain no node provenance at all. The result is a benchmark that can silently measure the same weeks-old binary while presenting a reassuringly flat trend. Needs imagePullPolicy: Always (or the immutable :<git describe> tag this same step already pushes) on the Argo side.

  2. K6_VUS/K6_DURATION outrank ci.json (nightly.sh:69) — unchanged from last round. k6's precedence is config file → script options → env vars → CLI flags, so those two sit above --config. Dropping warmup.js also dropped the unset K6_VUS K6_DURATION that guarded it. Nothing in this repo sets them; if the Argo manifest does, the measured run silently isn't 50 VUs / 30s and no artifact says so. One line at the top of benchmark().

  3. ci.json thresholds are absolute with no off switch (ci.json:5-8) — unchanged. checks: rate==1 / http_req_failed: rate==0 fail a case on one bad response across 50 VUs × 30s × 23 cases; nightly_run then returns non-zero and Argo retries all 23. STRICT_CHECKS used to default to 0; thresholds can't be set from the environment and nightly.sh exposes no --no-thresholds. k6 writes --summary-export regardless, so the only thing these buy is the exit code.

  4. The PR trigger doesn't watch the image's build inputs (rpc-benchmark.yaml:4-7) — unchanged. Makefile, go.mod, go.sum, starknet-p2p-specs/go.mod are all COPY'd by the Dockerfile and none are watched, so breakage lands on main where push: evaluates true.

Nit

  1. The tags expression is doing list construction inside a string (build-image.yaml:65) — correct today, but a comma smuggled into an interpolation is easy to break later. The block form reads better and build-push-action drops empty list entries:
    tags: |
      ${{ env.DOCKER_REGISTRY }}/${{ env.REPO_DEV }}/juno:${{ steps.set_tag.outputs.DOCKER_IMAGE_TAG }}
      ${{ github.ref == 'refs/heads/main' && format('{0}/{1}/juno:main', env.DOCKER_REGISTRY, env.REPO_DEV) || '' }}
  2. The reuse decision doesn't read corpus.json (internal/corpus.sh:24-28) — unchanged. A case dir with a valid command and a missing or truncated corpus is "current" forever: reuse → missing corpus → repeat. [[ -s $case_dir/corpus.json ]] restores the self-healing.
  3. WARMUP_ITERATIONS is the last unvalidated knob (nightly.sh:33) — unchanged. abc → k6 rejects --iterations → all 23 warmups fail → all measurements skipped.

Not worth another inline comment, all previously raised and answered: the catalog-read-⇒-rm -rf shape in prune_removed_cases, argument dispatch and the output contract vs argocd#9780, old snapshot trees never reclaimed, rm -f "$summary" before each case, pruning skipped when any case fails, .staging/ never reaped, and the absent bench/rpc/nightly/ docs — nothing in the repo names SNAPSHOT_SHA256, CORPUS_ROOT, RESULTS_DIR, WARMUP_ITERATIONS, CONCURRENCY, or RPC_TIMEOUT, which is the whole Argo contract.

Note: as in all eight prior rounds, the sandbox blocked bash -n, test-nightly.sh, and docker, and web access was unavailable. I read the caller workflow (deploy-dev-and-test.yml) to confirm the github.ref semantics above; everything else is a reading review, and I executed nothing.
· branch danielntmd/benchmark-endpoints

Comment thread .github/workflows/build-image.yaml
Comment thread .github/workflows/build-image.yaml
@danielntmd
danielntmd merged commit d13929e into main Aug 25, 2026
31 checks passed
@danielntmd
danielntmd deleted the danielntmd/benchmark-endpoints branch August 25, 2026 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants